Skip to content

[libtiff] use builtin as backup so that tiff can be always part of asimage#21975

Open
ferdymercury wants to merge 4 commits intoroot-project:masterfrom
ferdymercury:btiff
Open

[libtiff] use builtin as backup so that tiff can be always part of asimage#21975
ferdymercury wants to merge 4 commits intoroot-project:masterfrom
ferdymercury:btiff

Conversation

@ferdymercury
Copy link
Copy Markdown
Collaborator

@ferdymercury ferdymercury commented Apr 20, 2026

No description provided.

@github-actions
Copy link
Copy Markdown

Test Results

    15 files      15 suites   2d 6h 2m 50s ⏱️
 3 811 tests  3 811 ✅ 0 💤 0 ❌
51 152 runs  51 152 ✅ 0 💤 0 ❌

Results for commit b97aa10.

@dpiparo
Copy link
Copy Markdown
Member

dpiparo commented Apr 21, 2026

This PR is good work. Is anybody needing to generate natively tiff images? The discussion is still open if gif needs to be supported after 6.40...

@ferdymercury
Copy link
Copy Markdown
Collaborator Author

ferdymercury commented Apr 21, 2026

Thanks!

Is anybody needing to generate natively tiff images?

Not me at the moment, but having an extra ROOT build option assymmetric from png jpeg bmp gif seems weird. Here we remove one ROOT build option at the price of a builtin backup (that seems reasonably easy). This way functionality will be more reproducible in case someone was using asimage_tiff locally but not present in binaries or so.

The discussion is still open if gif needs to be supported

In my opinion, saving as gif still makes sense when you want to do animated videos via TCanvas::SaveAs(c.gif++NN) that you can easily include in any webpage or in a PPT. Unless there is an alternative way of generating those :)

Comment thread builtins/libtiff/CMakeLists.txt Outdated
@ferdymercury
Copy link
Copy Markdown
Collaborator Author

@bellenot
could you take a look at what's the proper library name on Windows? (non-urgent) thanks!

@ferdymercury
Copy link
Copy Markdown
Collaborator Author

ferdymercury commented Apr 21, 2026

I tried locally and I got:

It's because the PR rebases the branch before building. (There was a recent change of the name of LZMA merged into master)

It can be solved in your local build by changing BUILTIN_LZMA to LZMA in builtins/libtiff/CMakeLists.txt:91

Or alternatively, rebasing the branch into master in your local.

Thanks!!

Comment thread builtins/libtiff/CMakeLists.txt Outdated
@bellenot
Copy link
Copy Markdown
Member

bellenot commented Apr 21, 2026

@ferdymercury yes, fixed by git pull, thanks. And see my review.
And BTW, there are two libraries generated:

C:\root-dev\build\x64\relwithdebinfo>dir builtins\LIBTIFF-prefix\lib
 Volume in drive C is Windows
 Volume Serial Number is 2AD7-7B7A

 Directory of C:\root-dev\build\x64\relwithdebinfo\builtins\LIBTIFF-prefix\lib

21/04/2026  16:06    <DIR>          .
21/04/2026  16:06    <DIR>          ..
21/04/2026  16:06    <DIR>          cmake
21/04/2026  16:06    <DIR>          pkgconfig
21/04/2026  16:06         3,231,002 tiff.lib
21/04/2026  16:06           118,232 tiffxx.lib
               2 File(s)      3,349,234 bytes
               4 Dir(s)  168,042,983,424 bytes free

I don't know what tiffxx.lib is and if it's needed. We'll see...

Comment thread builtins/libtiff/CMakeLists.txt
@ferdymercury
Copy link
Copy Markdown
Collaborator Author

I don't know what tiffxx.lib is and if it's needed. We'll see...

Thanks, that ON by default, it's a library that enables C++ stream API building (requires C++ compiler).
I have disabled it now.

Thanks for the review !

Once it compiles, if you can run the test "simpleImages" from this branch, I am curious to see if Windows got correctly the JPEG library path passed.

@bellenot
Copy link
Copy Markdown
Member

C:\root-dev\build\x64\relwithdebinfo>ctest -VV -C RelWithDebInfo -R simpleImages
UpdateCTestConfiguration  from :C:/root-dev/build/x64/relwithdebinfo/DartConfiguration.tcl
Parse Config file:C:/root-dev/build/x64/relwithdebinfo/DartConfiguration.tcl
Test project C:/root-dev/build/x64/relwithdebinfo
Constructing a list of tests
Ignore test: test-tcollex
Ignore test: roottest-root-rint-TabCom
Ignore test: roottest-root-rint-BackslashNewline
Done constructing a list of tests
Updating test list for fixtures
Added 0 tests to meet fixture requirements
Checking test dependency graph...
Checking test dependency graph end
test 1829
    Start 1829: roottest-root-graf-simpleImages

1829: Test command: "C:\Program Files\CMake\bin\cmake.exe" "-DCMD=C:/root-dev/build/x64/relwithdebinfo/bin/root.exe^-e^#define ClingWorkAroundMissingDynamicScope^-e^#define ClingWorkAroundUnnamedInclude^-e^#define ClingWorkAroundMissingSmartInclude^-e^#define ClingWorkAroundNoDotInclude^-e^#define ClingWorkAroundMissingAutoLoadingForTemplates^-e^#define ClingWorkAroundTClassUpdateDouble32^-e^#define ClingWorkAroundAutoParseDeclaration^-e^#define ClingWorkAroundMissingUnloading^-e^#define ClingWorkAroundBrokenUnnamedReturn^-e^#define ClingWorkAroundUnnamedDetection2^-e^#define ClingWorkAroundNoPrivateClassIO^-e^#define ClingWorkAroundUnloadingVTABLES^-e^gSystem->SetBuildDir("C:/root-dev/build/x64/relwithdebinfo/roottest/root/graf",true)^-e^gSystem->AddDynamicPath("C:/root-dev/build/x64/relwithdebinfo/roottest/root/graf")^-e^gROOT->SetMacroPath("C:/root-dev/git/master/roottest/root/graf")^-e^gInterpreter->AddIncludePath("-IC:/root-dev/build/x64/relwithdebinfo/roottest/root/graf")^-e^gSystem->AddIncludePath("-IC:/root-dev/build/x64/relwithdebinfo/roottest/root/graf")^-q^-b^C:/root-dev/git/master/roottest/root/graf/simpleImages.C" "-DOUT=C:/root-dev/build/x64/relwithdebinfo/roottest/root/graf/simpleImages.log" "-DOUTREF=C:/root-dev/git/master/roottest/root/graf/simpleImages.ref" "-DCWD=C:/root-dev/build/x64/relwithdebinfo/roottest/root/graf" "-DDIFFCMD=C:/Python314/python.exe^C:/root-dev/git/master/roottest/scripts/custom_diff.py" "-DCHECKOUT=true" "-DCHECKERR=true" "-DSYS=C:/root-dev/build/x64/relwithdebinfo" "-DENV=PYTHONPATH=C:\root-dev\build\x64\relwithdebinfo\bin" "-P" "C:/root-dev/git/master/cmake/modules/RootTestDriver.cmake"
1829: Working Directory: C:/root-dev/build/x64/relwithdebinfo/roottest/root/graf
1829: Environment variables:
1829:  ROOT_HIST=0
1829: Test timeout computed to be: 300
1829: -- TEST COMMAND --
1829: cd C:/root-dev/build/x64/relwithdebinfo/roottest/root/graf
1829: C:/root-dev/build/x64/relwithdebinfo/bin/root.exe -e '#define ClingWorkAroundMissingDynamicScope' -e '#define ClingWorkAroundUnnamedInclude' -e '#define ClingWorkAroundMissingSmartInclude' -e '#define ClingWorkAroundNoDotInclude' -e '#define ClingWorkAroundMissingAutoLoadingForTemplates' -e '#define ClingWorkAroundTClassUpdateDouble32' -e '#define ClingWorkAroundAutoParseDeclaration' -e '#define ClingWorkAroundMissingUnloading' -e '#define ClingWorkAroundBrokenUnnamedReturn' -e '#define ClingWorkAroundUnnamedDetection2' -e '#define ClingWorkAroundNoPrivateClassIO' -e '#define ClingWorkAroundUnloadingVTABLES' -e 'gSystem->SetBuildDir("C:/root-dev/build/x64/relwithdebinfo/roottest/root/graf",true)' -e 'gSystem->AddDynamicPath("C:/root-dev/build/x64/relwithdebinfo/roottest/root/graf")' -e 'gROOT->SetMacroPath("C:/root-dev/git/master/roottest/root/graf")' -e 'gInterpreter->AddIncludePath("-IC:/root-dev/build/x64/relwithdebinfo/roottest/root/graf")' -e 'gSystem->AddIncludePath("-IC:/root-dev/build/x64/relwithdebinfo/roottest/root/graf")' -q -b C:/root-dev/git/master/roottest/root/graf/simpleImages.C
1829: -- BEGIN TEST OUTPUT --
1829: Processing C:/root-dev/git/master/roottest/root/graf/simpleImages.C...
1829: Info in <TCanvas::Print>: jpg file f.jpeg has been created
1829: Info in <TCanvas::Print>: png file f.png has been created
1829: Info in <TCanvas::Print>: gif file f.gif has been created
1829: Info in <TCanvas::Print>: bmp file f.bmp has been created
1829: TIFFSetField: f.tiff: Unknown pseudo-tag 65537.
1829: TIFFSetField: f.tiff: Unknown pseudo-tag 65538.
1829: f.tiff: JPEG compression support is not configured.
1829: Info in <TCanvas::Print>: tiff file f.tiff has been created
1829: (int) 0
1829:
1829: -- END TEST OUTPUT --
1829: -- BEGIN OUTDIFF OUTPUT --
1829: --- C:/root-dev/git/master/roottest/root/graf/simpleImages.ref    Tue Apr 21 15:39:10 2026
1829: +++ C:/root-dev/build/x64/relwithdebinfo/roottest/root/graf/simpleImages.log      Tue Apr 21 16:27:41 2026
1829: @@ -2,5 +2,8 @@
1829:  Info in <TCanvas::Print>: png file f.png has been created
1829:  Info in <TCanvas::Print>: gif file f.gif has been created
1829:  Info in <TCanvas::Print>: bmp file f.bmp has been created
1829: +TIFFSetField: f.tiff: Unknown pseudo-tag 65537.
1829: +TIFFSetField: f.tiff: Unknown pseudo-tag 65538.
1829: +f.tiff: JPEG compression support is not configured.
1829:  Info in <TCanvas::Print>: tiff file f.tiff has been created
1829:  (int) 0
1829:
1829: -- END OUTDIFF OUTPUT --
1829: CMake Error at C:/root-dev/git/master/cmake/modules/RootTestDriver.cmake:280 (message):
1829:   compare 'stdout' error: 1
1829:
1829:
1/1 Test #1829: roottest-root-graf-simpleImages ...***Failed   12.35 sec

0% tests passed, 1 tests failed out of 1

Total Test time (real) =  13.50 sec

The following tests FAILED:
        1829 - roottest-root-graf-simpleImages (Failed)
Errors while running CTest

@ferdymercury
Copy link
Copy Markdown
Collaborator Author

Thanks! Can you also look at the output log of the configure step of tiff ? I fear that it's not finding the builtin_jpeg path
It should say:
-- JPEG support: Requested:ON Availability:TRUE Support:TRUE

(The CMakeLists is passing JPEG_INCLUDE_DIR to the external project but maybe incorrectly)

@bellenot
Copy link
Copy Markdown
Member

Alright:

--   JPEG support:                       Requested:OFF Availability:FALSE Support:FALSE

Comment thread builtins/libtiff/CMakeLists.txt
@bellenot
Copy link
Copy Markdown
Member

Here is the full thig:

--  Support for external codecs:
--   ZLIB support:                       Requested:ON Availability:TRUE Support:TRUE
--   libdeflate support:                 Requested:OFF Availability:FALSE Support:FALSE
--   Pixar log-format algorithm:         Requested:OFF Availability:TRUE Support:FALSE
--   JPEG support:                       Requested:OFF Availability:FALSE Support:FALSE
--   Old JPEG support:                   Requested:OFF Availability:FALSE Support:FALSE (Depends on JPEG Support)
--   JPEG 8/12 bit dual mode:            Requested:OFF Availability:FALSE Support:FALSE
--   ISO JBIG support:                   Requested:OFF Availability:FALSE Support:FALSE
--   LERC support:                       Requested:OFF Availability:FALSE Support:FALSE
--   LZMA2 support:                      Requested:OFF Availability:FALSE Support:FALSE
--   ZSTD support:                       Requested:ON Availability:TRUE Support:TRUE
--   WEBP support:                       Requested:OFF Availability:FALSE Support:FALSE

Comment thread builtins/libtiff/CMakeLists.txt
@ferdymercury
Copy link
Copy Markdown
Collaborator Author

I've pushed a fix, in case you could retry the configure step now. Thks!

@bellenot
Copy link
Copy Markdown
Member

Still no good:

-- Could NOT find JPEG (missing: JPEG_LIBRARY JPEG_INCLUDE_DIR) 
[...]
--  Support for external codecs:
--   ZLIB support:                       Requested:ON Availability:TRUE Support:TRUE
--   libdeflate support:                 Requested:OFF Availability:FALSE Support:FALSE
--   Pixar log-format algorithm:         Requested:OFF Availability:TRUE Support:FALSE
--   JPEG support:                       Requested:ON Availability:FALSE Support:FALSE
--   Old JPEG support:                   Requested:OFF Availability:FALSE Support:FALSE (Depends on JPEG Support)
--   JPEG 8/12 bit dual mode:            Requested:OFF Availability:FALSE Support:FALSE
--   ISO JBIG support:                   Requested:OFF Availability:FALSE Support:FALSE
--   LERC support:                       Requested:OFF Availability:FALSE Support:FALSE
--   LZMA2 support:                      Requested:OFF Availability:FALSE Support:FALSE
--   ZSTD support:                       Requested:ON Availability:TRUE Support:TRUE
--   WEBP support:                       Requested:OFF Availability:FALSE Support:FALSE
-- 

@bellenot
Copy link
Copy Markdown
Member

Looking into it

@bellenot
Copy link
Copy Markdown
Member

bellenot commented Apr 21, 2026

So this works:

diff --git a/builtins/libjpeg/CMakeLists.txt b/builtins/libjpeg/CMakeLists.txt
index 7af532e2fa8..6148249122e 100644
--- a/builtins/libjpeg/CMakeLists.txt
+++ b/builtins/libjpeg/CMakeLists.txt
@@ -61,7 +61,7 @@ add_dependencies(JPEG::JPEG BUILTIN_LIBJPEG)

 # Set the canonical output of find_package according to
 # https://cmake.org/cmake/help/latest/manual/cmake-developer.7.html#standard-variable-names
-set(JPEG_INCLUDE_DIRS ${${ROOT_LIBJPEG_PREFIX}/include} PARENT_SCOPE)
+set(JPEG_INCLUDE_DIR ${${ROOT_LIBJPEG_PREFIX}/include} PARENT_SCOPE)
 set(JPEG_LIBRARIES ${ROOT_LIBJPEG_LIBRARY} PARENT_SCOPE)
 set(JPEG_FOUND TRUE PARENT_SCOPE)
 set(JPEG_VERSION ${ROOT_LIBJPEG_VERSION} PARENT_SCOPE)

And keep these line in the ExternalProject_Add of builtins/libtiff/CMakeLists.txt

    -DJPEG_LIBRARY=${JPEG_LIBRARIES}
    -DJPEG_INCLUDE_DIR=${JPEG_INCLUDE_DIR}

The test pass:

1829: -- BEGIN TEST OUTPUT --
1829: Processing C:/root-dev/git/master/roottest/root/graf/simpleImages.C...
1829: Info in <TCanvas::Print>: jpg file f.jpeg has been created
1829: Info in <TCanvas::Print>: png file f.png has been created
1829: Info in <TCanvas::Print>: gif file f.gif has been created
1829: Info in <TCanvas::Print>: bmp file f.bmp has been created
1829: Info in <TCanvas::Print>: tiff file f.tiff has been created
1829: (int) 0
1829:
1829: -- END TEST OUTPUT --
1/1 Test #1829: roottest-root-graf-simpleImages ...   Passed    4.13 sec

Looking now for side effects...

@ferdymercury
Copy link
Copy Markdown
Collaborator Author

And using directly

-DJPEG_INCLUDE_DIR=${JPEG_INCLUDE_DIRS}

without modifying the libjpeg builtin CMakeLists would fail? (that would be weird)

@bellenot
Copy link
Copy Markdown
Member

And using directly

-DJPEG_INCLUDE_DIR=${JPEG_INCLUDE_DIRS}

without modifying the libjpeg builtin CMakeLists would fail? (that would be weird)

Well, it was failing originally with -DJPEG_INCLUDE_DIR=${JPEG_INCLUDE_DIRS}. But I'll check again...

@ferdymercury
Copy link
Copy Markdown
Collaborator Author

ferdymercury commented Apr 21, 2026

Well, it was failing originally with -DJPEG_INCLUDE_DIR=${JPEG_INCLUDE_DIRS}. But I'll check again...

Ahh, maybe it's because this is a typo with double dollars?

-set(JPEG_INCLUDE_DIRS ${${ROOT_LIBJPEG_PREFIX}/include} PARENT_SCOPE)

should rather be

+set(JPEG_INCLUDE_DIRS ${ROOT_LIBJPEG_PREFIX}/include PARENT_SCOPE)

?

@bellenot
Copy link
Copy Markdown
Member

should rather be

+set(JPEG_INCLUDE_DIRS ${ROOT_LIBJPEG_PREFIX}/include PARENT_SCOPE)

?

Good catch! With this, both -DJPEG_INCLUDE_DIR=${JPEG_INCLUDE_DIRS} and -DJPEG_INCLUDE_DIR=${JPEG_INCLUDE_DIR} should work.
I added message(STATUS "JPEG_INCLUDE_DIR = ${JPEG_INCLUDE_DIR} - JPEG_INCLUDE_DIRS = ${JPEG_INCLUDE_DIRS}") in the CMakeLists.txt:

-- JPEG_INCLUDE_DIR = C:/root-dev/build/x64/relwithdebinfo/builtins/LIBJPEG-prefix/include - JPEG_INCLUDE_DIRS = C:/root-dev/build/x64/relwithdebinfo/builtins/LIBJPEG-prefix/include

Let me check both options...

@bellenot
Copy link
Copy Markdown
Member

bellenot commented Apr 21, 2026

So I confirm that with set(JPEG_INCLUDE_DIRS ${ROOT_LIBJPEG_PREFIX}/include PARENT_SCOPE), both -DJPEG_INCLUDE_DIR=${JPEG_INCLUDE_DIRS} and -DJPEG_INCLUDE_DIR=${JPEG_INCLUDE_DIR} work.
Sorry for having overlooked at this...

@ferdymercury
Copy link
Copy Markdown
Collaborator Author

So I confirm that with set(JPEG_INCLUDE_DIRS ${ROOT_LIBJPEG_PREFIX}/include PARENT_SCOPE), both -DJPEG_INCLUDE_DIR=${JPEG_INCLUDE_DIRS} and -DJPEG_INCLUDE_DIR=${JPEG_INCLUDE_DIR} work.

Great, thanks! Yes, I believe that's because JPEG_INCLUDE_DIR is set elsewhere:

cmake/modules/SearchInstalledSoftware.cmake: get_target_property(JPEG_INCLUDE_DIR JPEG::JPEG INTERFACE_INCLUDE_DIRECTORIES)

so both are interchangeable now.

@ferdymercury ferdymercury marked this pull request as ready for review April 21, 2026 16:16
@ferdymercury ferdymercury requested a review from dpiparo as a code owner April 21, 2026 16:16
@ferdymercury ferdymercury requested a review from bellenot April 21, 2026 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants